feat: add DAG-JOSE implementation #177
Conversation
This is an implementation of DAG-JOSE. There are also a few minor changes to existing code. * An update to the dag-cbor-derive code in order to allow usage of the macro from within this repo. * Add a `Bytes` type to dag-cbor to make declaring bytes within a struct more straightforward. * A dag-jose feature There are some things left to do before I would consider this PR ready. However I would like feedback on the general design first. You can see its usage in the fixtures.rs test case. * Provide complete examples in docs * Determine what to do about the fixture tests. We likely want to move that logic to https://github.com/ipld/codec-fixtures * Add Ipld conversion to the types to avoid round-tripping through CBOR for JSON encoding.
| /// Bytes is sequence of byte values. | ||
| /// | ||
| /// Implements Encode and Decode to/from CBOR byte strings | ||
| pub type Bytes = Box<[u8]>; |
There was a problem hiding this comment.
I'm a little confused as to why this is needed. It seems like Vec is covered. Also why Box<[u8]> and not Vec.
There was a problem hiding this comment.
It stems from here https://github.com/ipld/libipld/blob/master/dag-cbor/src/decode.rs#L297 and here https://github.com/ipld/libipld/blob/master/dag-cbor/src/decode.rs#L321
Using a Vec<u8> creates an IPLD list of u8 values, where using Box<[u8]> creates an IPLD byte sequence.
We could change the cbor Decode implementations but this was the smallest change I could make to existing code. Additionally we could try and specialize the implementation but that required Rust nightly.
There was a problem hiding this comment.
LIttle odd that they didn't handle Vec as a byte string, but is there a reason we need a ByteString here, and an array of u8 isn't sufficient. A large number of libraries work on Vec or bytes::Bytes, so to use this, they'll have to be regularly calling into_boxed_slice() to work with the types specified here.
dag-pb actually uses bytes::Bytes. Oddly, it also returns a Box<[u8]> for the payload here https://github.com/ipld/libipld/blob/master/dag-pb/src/codec.rs#L78 rather than the Vec or a Bytes structure.
| let protected = base64_url::decode("eyJhbGciOiJFZERTQSJ9").unwrap(); | ||
| let signature = base64_url::decode("-_9J5OZcl5lVuRlgI1NJEzc0FqEb6_2yVskUaQPducRQ4oe-N5ynCl57wDm4SPtm1L1bltrphpQeBOeWjVW1BQ").unwrap(); | ||
| ( | ||
| payload.into_boxed_slice(), |
There was a problem hiding this comment.
Seems like this should just be to_vec().
| ) | ||
| } | ||
| fn fixture_jws_base64( | ||
| payload: &Box<[u8]>, |
|
This is not an in-depth review, but a few general notes. The way I propose to use DAG-CBOR is via https://crates.io/crates/serde_ipld_dagcbor. It is based on Serde, this way you get access to the Serde ecosystem. Whenever you define Serde (de)serialization for some type, you can transform it into IPLD, that's a huge win. Given that, I hope we can move away from the current Given all that I propose keeping the DAG-JOSE codec in a separate repo. If changes to |
It would be great to have DAG-JOSE in the |
|
Thanks for the comments. I have implemented the changes against a new repo. ceramicnetwork/rust-dag-jose#1 |
This is an implementation of DAG-JOSE. There are also a few minor changes to existing code.
Bytestype to dag-cbor to make declaring bytes within a struct more straightforward.There are some things left to do before I would consider this PR ready. However I would like feedback on the general design first. You can see its usage in the fixtures.rs test case.